-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ (go/v3,go/v4-alpha): instead of call script to install kustomize download it first #3187
✨ (go/v3,go/v4-alpha): instead of call script to install kustomize download it first #3187
Conversation
|
/hold As for the CLA, did that change somehow? I thought the IBM github org was a signatory to the CLA and my membership is public. |
f14a596
to
dc74616
Compare
/retest |
dc74616
to
7779c40
Compare
d2b583d
to
bb4884a
Compare
bb4884a
to
2862464
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like this would result in the experience for a user being unchanged but not being flagged by automated security alerts. I'm fine with this change going in.
Will wait to add any labels so the bot doesn't auto merge and allow for further discussion
todo: backport this to v3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jberkhahn,
Could you please add in the description the alerts/the output that you are facing which motivates you push this PR?
todo: backport this to v3
You do not backport. You need to change the makefile template for the go/v3 plugin and run make generate to ensure that testdata is updated as well.
The security management software IBM runs flags this line as a vulnerability and kill -9's it whenever you try to execute it (by running make bundle-build or using any of the operator sdk commands that use that make task). As a result, IBMers currently can't make use of that functionality. We discussed this a bit internally and at the operator sdk meeting, and we think this is the most light-weight solution. I was going to attend the next kubebuilder meeting and discuss this, but I guess I just missed one so it's not for 2 weeks? |
Hi @jberkhahn, Regards #3187 (comment). The meeting is biweekly. The next one will be on Feb 9. But we do not need to bring that to the meeting.
Regarding the proposed change, I do not have any strong opinion about it. Could you please update the go/v3 plugin (same boilerplate) and make generate? |
Signed-off-by: jberkhahn <jaberkha@us.ibm.com>
2862464
to
fb1ef1d
Compare
Ok, updated this to include v3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the sake of consistency, do we also need to update here:
https://github.com/kubernetes-sigs/kubebuilder/blob/v3.9.0/Makefile#L90
golangci-lint:
@[ -f $(GOLANGCI_LINT) ] || { \
set -e ;\
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(shell dirname $(GOLANGCI_LINT)) v1.50.1 ;\
}
Hi @Kavinjsir Regards #3187 (review)
We do not need to update this one because it is ONLY used by those that contributes with Kubebuilder. therefore, it will not affected the end-users. |
@jberkhahn I updated the title accordingly. |
updated the description |
/hold cancel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, everettraven, jberkhahn, varshaprasad96 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
The security management software IBM runs flags this line as a vulnerability and kill -9's it whenever you try to execute it (by running make bundle-build or using any of the operator sdk commands that use that make task). As a result, IBMers currently can't make use of that functionality. This changes up the line a little to be more palatable to our security software.